feat(#10706): add bulk operation framework and contact hierarchy delete#11236
Conversation
Introduce the @medic/bulk-operations shared library holding the document model that the bulk operation framework shares across delete, move and merge: the log document (medic-logs) read by the polling endpoint, and the per-action documents (medic-sentinel) that Sentinel processes in batches. buildBulkOperation assembles both from per-action operation lists. The per-item params live in a base64 json attachment so advancing an action's cursor does not rewrite the whole list.
Add GET /api/v1/bulk-operations/{id}, which returns the bulk operation log
document from the medic-logs database so a caller can poll the progress of
a delete, move or merge it started.
The service only resolves ids carrying the bulk-operation prefix, so the
endpoint cannot be used to read the other log documents that share the
database. Online users only; missing or non-bulk-operation ids return 404.
Add queue(actionOperations) to the bulk-operations service. It builds the log and action documents and writes the log to medic-logs first, then one action document per type to medic-sentinel, skipping empty action groups, and returns the bulk operation id.
Add a medic-sentinel changes-feed listener that queues bulk-operation-action docs (existing ones on startup, new ones from the feed) and a processor that works through each in batches, tracking the cursor, then records the result on the log doc and removes the action doc. Includes the set-contact and delete-user handlers; the archive handler follows with the archiving work. Adds the medic-logs handle and getAttachment to sentinel's db.
Add DELETE /api/v1/person/{id} and /api/v1/place/{id} over a shared
delete-contact service that gathers the subtree, its reports (matched by uuid
and shortcode), the primary-contact places to clear, and the linked users, then
queues the bulk operation and returns the breakdown. Gated on
can_delete_contact_hierarchy, with delete_users (requires can_delete_users) and
dry_run query params.
Compute the delete-user operations once (lowering the function's cognitive complexity) and drop an unreachable empty-subject guard.
Exercise the feed change handler, dedup, the empty-batch guard and a failing action, use specific assertions, and mark the changes-feed error retry (live feed infrastructure) as ignored for coverage.
…ld passes The docs build's JSDoc parser rejected the TypeScript-style optional-property type on deleteContactHierarchy's @returns; replace it and the record-in-generic param types with plain JSDoc types.
Add can_delete_contact_hierarchy to the default permissions with no role, so it is assignable and shows in the admin UI. Admins still pass the gate by default.
…anagement-operations' into 10706-delete-contact-hierarchy
The person and place DELETE endpoints delete a hierarchy the same way, so their controller handlers and @openapi blocks were near-identical. Move the shared handler into the delete-contact service (the controllers now just point their delete at it) and pull the delete_users/dry_run params and the response shapes into shared OpenAPI components referenced from both.
The person and place DELETE endpoints shared one handler and neither checked the target's type, so a place id sent to the person endpoint would have deleted the whole place hierarchy. Each endpoint now passes its own type getter to the shared handler, which rejects an id of the wrong type with a 404, the same way the GET endpoints already do. The gather-and-queue logic stays type-agnostic in deleteContactHierarchy.
jkuester
left a comment
There was a problem hiding this comment.
@vikrantwiz02 this is great! Your code structure is very clear and tight and everything follows the design! ❤️
I am going to post my first-pass comments now, so you can start on them (have not had a change to look at the tests yet). Also, just disregard the comments starting with TODO 😅 . I am still thinking through some of the failure cases in Sentinel and do not have my thoughts completely formed yet on if we need to do anything different for some of that...
| const failed = []; | ||
| for (const op of batch) { | ||
| try { | ||
| await userManagement.deleteUser(op.id.replace(PREFIXES.COUCH_USER, '')); |
There was a problem hiding this comment.
Probably need to have an explicit check that op.id is valued and just skip straight to the failure if it does not.
There was a problem hiding this comment.
Done (added to both handlers).
| const BATCH_SIZE = 100; | ||
| const RETRY_TIMEOUT = 60000; | ||
|
|
||
| // set-contact: point a place's `contact` reference at a new value (or clear it when `contact` is |
There was a problem hiding this comment.
Technically a report also has a contact property that could be updated with this action. (Not being used in our current workflow, but could possibly be leveraged in the future.)
There was a problem hiding this comment.
Left it for now, can add updating the report's contact ref as a follow-up if we want it.
| const doc = docsById[op.id]; | ||
| const currentContactId = doc && (doc.contact?._id || doc.contact); | ||
| if (!doc || currentContactId !== op.current_contact_id) { | ||
| failed.push(op); |
There was a problem hiding this comment.
Can we log a message when failing the operation? Would be ideal to include the action id and indicate if it failed because the doc was missing or because the contact id changed.
There was a problem hiding this comment.
Done, logs the action id and whether it was a missing doc or a changed contact.
| try { | ||
| await userManagement.deleteUser(op.id.replace(PREFIXES.COUCH_USER, '')); | ||
| } catch (err) { | ||
| logger.error(`bulk-operations: failed to delete user ${op.id}: %o`, err); |
There was a problem hiding this comment.
Can we also include the action id in the log message?
…delete - move the bulk-operation constants into @medic/constants and fold the doc model into the api service, dropping the @medic/bulk-operations shared-lib - split the sentinel processor into per-action files (set-contact, delete-user) - queue action ids on the feed instead of whole docs; register the feed before loading the initial queue - match linked users by contact_id as well as facility_id - run the archive action last, after the references to those contacts are cleared - keep the delete gather logic internal, tested through the handler - rename the response breakdown to summary and flesh out the OpenAPI schema
…n actions Name the set-contact and delete-user handler functions (they were anonymous exported arrows), and reduce processAction's cognitive complexity by pulling the action fetch and the batch loop into helpers.
…points Covers permissions, dry-run summaries and the type/linked-user error cases for DELETE /api/v1/person, DELETE /api/v1/place and GET /api/v1/bulk-operations. The removal assertions poll the bulk operation to completion, which needs the archive action handler from medic#6615, so those are skipped until it lands.
…narcloud The poll-to-removal tests depend on the medic#6615 archive handler and were parked in describe.skip, which trips sonar's "no skipped tests" rule. They land with that handler instead. Also make the new describe callbacks synchronous.
Description
The server-side implementation of contact-hierarchy delete for #10706. A
DELETEon the person and place endpoints gathers everything the delete touches and queues a bulk operation that Sentinel executes asynchronously, so the slow work runs off the request. The bulk operation framework is generic: delete is its first consumer, and move and merge will reuse it.What's here, one commit per layer:
@medic/constants.medic-logsand one action doc per type tomedic-sentinel.GET /api/v1/bulk-operations/{id}: poll an operation's status.medic-sentinelchanges-feed listener plus a batch/cursor processor, with theset-contactanddelete-userhandlers.DELETE /api/v1/person/{id}and/api/v1/place/{id}: gated oncan_delete_contact_hierarchy, withdelete_users(requirescan_delete_users, else a400when linked users exist) anddry_runquery params; each endpoint rejects an id of the wrong type with a404; returns a summary of the changes plus the operation id.The
archiveaction handler (copy to the archive database and purge) builds on the archiving work in #6615 / #11139, so it lands once that merges; the framework and the other handlers stand on their own.Endpoint behaviour is covered by integration tests: the permissions, the dry-run summaries and the type/linked-user error cases. The removal assertions poll the operation to completion, so they land with the
archivehandler.Supersedes the earlier delete PRs, now closed and folded in here: #11163, #11167, #11172, #11176 (the first synchronous attempt) and #11222, #11223 (the split framework pieces).
Part of #10706
Notes
The gather-and-queue logic lives once in
deleteContactHierarchyand is contact-type-agnostic, a person delete is just the leaf case of a place-subtree delete. The person and place endpoints each call a shared handler with their own type, and the handler checks the target is that type before doing anything, so a place id sent to/person/{id}is rejected with a 404 instead of deleting the place hierarchy, and vice versa. Thedelete_users/dry_runparams and the responses are shared OpenAPI components referenced from both.Code review checklist
License
The software is provided under AGPL-3.0. Contributions to this project are accepted under the same license.